fix: Response body for post #484
Conversation
There was a problem hiding this comment.
Hi @VivekHub97 ,
Thanks a lot for the implementation.
Actually, this should be fixed in core [1] and [2] first and then at the Controller side, the return type here in [1] and [2] is void but it should be SubmodelElement, and then at the HttpController [3] and [4] respectively for Repo and Service, it should be like below:
@Override
public ResponseEntity<SubmodelElement> postSubmodelElementSubmodelRepo(Base64UrlEncodedIdentifier submodelIdentifier, @Valid SubmodelElement body) {
SubmodelElement createdSME = repository.createSubmodelElement(submodelIdentifier.getIdentifier(), body);
return new ResponseEntity<SubmodelElement>(createdSME, HttpStatus.CREATED);
}Inside SubmodelRepoHttpController [3] the method postSubmodelElementByPathSubmodelRepo is not fixed.
Following is the summary:
- Update interfaces of SubmodelRepository [1] and SubmodelService [2] for createSubmodelElement methods (there are multiple overloaded methods) such that the return type is SME instead of void.
- Then handle this appropriately wherever these interfaces are implemented possibly in all implementations (Crud*Repo) and all features.
- Update HTTP controllers for SMRepo [3] and SMService [4] like I defined above
- Update the test cases to additionally check the creation response wherever required. (Maybe do this first to achieve Test Driven Development).
[1] basyx-java-server-sdk/basyx.submodelrepository/basyx.submodelrepository-core/src/main/java/org/eclipse/digitaltwin/basyx/submodelrepository/SubmodelRepository.java at main · eclipse-basyx/basyx-java-server-sdk (github.com)
[2] basyx-java-server-sdk/basyx.submodelservice/basyx.submodelservice-core/src/main/java/org/eclipse/digitaltwin/basyx/submodelservice/SubmodelService.java at main · eclipse-basyx/basyx-java-server-sdk (github.com)
[3] basyx-java-server-sdk/basyx.submodelrepository/basyx.submodelrepository-http/src/main/java/org/eclipse/digitaltwin/basyx/submodelrepository/http/SubmodelRepositoryApiHTTPController.java at main · eclipse-basyx/basyx-java-server-sdk (github.com)
[4] basyx-java-server-sdk/basyx.submodelservice/basyx.submodelservice-http/src/main/java/org/eclipse/digitaltwin/basyx/submodelservice/http/SubmodelServiceHTTPApiController.java at main · eclipse-basyx/basyx-java-server-sdk (github.com)
VivekHub97
force-pushed
the
bugFix_body
branch
from
e7d1b97 to
52e3ab5
Compare
| public SubmodelElement createSubmodelElement(String submodelId, SubmodelElement smElement) { | ||
| SubmodelService submodelService = getSubmodelServiceOrThrow(submodelId); | ||
|
|
||
| submodelService.createSubmodelElement(smElement); | ||
|
|
||
| updateSubmodel(submodelId, submodelService.getSubmodel()); | ||
| return smElement; | ||
| } |
There was a problem hiding this comment.
| public SubmodelElement createSubmodelElement(String submodelId, SubmodelElement smElement) { | |
| SubmodelService submodelService = getSubmodelServiceOrThrow(submodelId); | |
| submodelService.createSubmodelElement(smElement); | |
| updateSubmodel(submodelId, submodelService.getSubmodel()); | |
| return smElement; | |
| } | |
| public SubmodelElement createSubmodelElement(String submodelId, SubmodelElement smElement) { | |
| SubmodelService submodelService = getSubmodelServiceOrThrow(submodelId); | |
| SubmodelElement createdSME = submodelService.createSubmodelElement(smElement); | |
| updateSubmodel(submodelId, submodelService.getSubmodel()); | |
| return createdSME; | |
| } |
| public SubmodelElement createSubmodelElement(String submodelId, String idShortPath, SubmodelElement smElement) throws ElementDoesNotExistException { | ||
| SubmodelService submodelService = getSubmodelServiceOrThrow(submodelId); | ||
|
|
||
| submodelService.createSubmodelElement(idShortPath, smElement); | ||
|
|
||
| updateSubmodel(submodelId, submodelService.getSubmodel()); | ||
| return smElement; | ||
| } |
There was a problem hiding this comment.
| public SubmodelElement createSubmodelElement(String submodelId, String idShortPath, SubmodelElement smElement) throws ElementDoesNotExistException { | |
| SubmodelService submodelService = getSubmodelServiceOrThrow(submodelId); | |
| submodelService.createSubmodelElement(idShortPath, smElement); | |
| updateSubmodel(submodelId, submodelService.getSubmodel()); | |
| return smElement; | |
| } | |
| public SubmodelElement createSubmodelElement(String submodelId, String idShortPath, SubmodelElement smElement) throws ElementDoesNotExistException { | |
| SubmodelService submodelService = getSubmodelServiceOrThrow(submodelId); | |
| SubmodelElement createdSME = submodelService.createSubmodelElement(idShortPath, smElement); | |
| updateSubmodel(submodelId, submodelService.getSubmodel()); | |
| return createdSME; | |
| } |
| @@ -99,17 +99,19 @@ public void setSubmodelElementValue(String submodelId, String idShortPath, Submo | |||
| } | |||
|
|
|||
| @Override | |||
| public void createSubmodelElement(String submodelId, SubmodelElement smElement) { | |||
| public SubmodelElement createSubmodelElement(String submodelId, SubmodelElement smElement) { | |||
| decorated.createSubmodelElement(submodelId, smElement); | |||
| SubmodelElement submodelElement = decorated.getSubmodelElement(submodelId, smElement.getIdShort()); | |||
There was a problem hiding this comment.
Ideally, we can get rid of this getSME operation and use the createSME return.
| SubmodelElement submodelElement = decorated.getSubmodelElement(submodelId, smElement.getIdShort()); | |
| SubmodelElement submodelElement = decorated.createSubmodelElement(submodelId, smElement); |
| } | ||
|
|
||
| @Override | ||
| public void createSubmodelElement(String submodelId, String idShortPath, SubmodelElement smElement) throws ElementDoesNotExistException { | ||
| public SubmodelElement createSubmodelElement(String submodelId, String idShortPath, SubmodelElement smElement) throws ElementDoesNotExistException { | ||
| decorated.createSubmodelElement(submodelId, idShortPath, smElement); | ||
| SubmodelElement submodelElement = decorated.getSubmodelElement(submodelId, idShortPath); |
|
|
||
| assertEquals(responseSubmodelElement, submodelElement); |
There was a problem hiding this comment.
Expected should be first parameter.
| assertEquals(responseSubmodelElement, submodelElement); | |
| assertEquals(submodelElement, responseSubmodelElement); |
| InMemorySubmodelService inMemorySubmodelService = getInMemorySubmodelService(); | ||
| inMemorySubmodelService.createSubmodelElement(submodelElement); | ||
| Submodel submodel = inMemorySubmodelService.getSubmodel(); | ||
| crudRepository.save(submodel); | ||
| return submodelElement; |
There was a problem hiding this comment.
Use the returned SME from line 108 and not return the SME received as a parameter.
| InMemorySubmodelService inMemorySubmodelService = getInMemorySubmodelService(); | ||
| inMemorySubmodelService.createSubmodelElement(idShortPath, submodelElement); | ||
| Submodel submodel = inMemorySubmodelService.getSubmodel(); | ||
| crudRepository.save(submodel); | ||
| return submodelElement; |
| @@ -718,6 +718,10 @@ private DefaultEntity createDummyEntityWithStatement(SubmodelElement submodelEle | |||
| private DefaultProperty createDummyProperty(String idShort) { | |||
| return new DefaultProperty.Builder().idShort(idShort).category("cat1").value("123").valueType(DataTypeDefXsd.INTEGER).build(); | |||
| } | |||
|
|
|||
| private SubmodelElement createDummySME(String idShort) { | |||
There was a problem hiding this comment.
Could you please explain the need of this? Because I can't find any reference to this private method in this class.
| @@ -93,19 +93,21 @@ public void setSubmodelElementValue(String idShortPath, SubmodelElementValue val | |||
| } | |||
|
|
|||
| @Override | |||
| public void createSubmodelElement(SubmodelElement submodelElement) { | |||
| public SubmodelElement createSubmodelElement(SubmodelElement submodelElement) { | |||
| decorated.createSubmodelElement(submodelElement); | |||
| SubmodelElement smElement = decorated.getSubmodelElement(submodelElement.getIdShort()); | |||
There was a problem hiding this comment.
We can get rid of this method and use the returned SME from line 97
| } | ||
|
|
||
| @Override | ||
| public void createSubmodelElement(String idShortPath, SubmodelElement submodelElement) throws ElementDoesNotExistException { | ||
| public SubmodelElement createSubmodelElement(String idShortPath, SubmodelElement submodelElement) throws ElementDoesNotExistException { | ||
|
|
||
| decorated.createSubmodelElement(idShortPath, submodelElement); | ||
|
|
||
| SubmodelElement smElement = decorated.getSubmodelElement(submodelElement.getIdShort()); |
fix: Response body for posting in submodel repository and submodel service
Closes #398
Closes #457